Skip to content

Fetch full documentation in code completion #2207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

a7medev
Copy link

@a7medev a7medev commented Jul 15, 2025

Depends on swiftlang/swift#82464


When resolving documentation for code completion items, we fetch full documentation through the newly added swiftide_completion_item_get_doc_full_copy SourceKitD function, if not found we fallback to brief documentation as before using swiftide_completion_item_get_doc_brief.

Note

Unlike brief documentation, SourceKitD doesn't cache full documentation for completion results to avoid bloating memory with a lot of large strings.

As of now, SourceKit-LSP doesn't cache completion item documentation either, should we introduce a new full documentation cache (e.g. using LRUCache)?

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool stuff. Excited to see this. I left a couple comments inline.

@a7medev a7medev requested a review from ahoppen July 16, 2025 22:30
@a7medev a7medev requested a review from ahoppen July 17, 2025 08:19
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me, the only open question is whether we want to return both the brief and the full documentation from the plugin.

Also: Could you format your changes using swift-format as described in https://github.com/swiftlang/sourcekit-lsp/blob/main/CONTRIBUTING.md#formatting?

@a7medev a7medev requested a review from ahoppen July 17, 2025 19:11
@a7medev
Copy link
Author

a7medev commented Jul 17, 2025

I have added one more functionality change.
In the initial implementation, I converted XML documentation to markdown using the same existing flow which includes the declaration as part of the documentation.

I found that existing LSP implementations (mainly Clang and TypeScript) don't show the declaration itself as part of the full documentation in completion (contrary to the hover request!) and I think the reason for this is that the completion item already describes the declaration and typically the popup showing the documentation is small so it makes sense to show the documentation comment right away.

@ahoppen @hamishknight Can you please recheck? 🙏🏼

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me. Thank you 🚀

@ahoppen
Copy link
Member

ahoppen commented Jul 18, 2025

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Jul 18, 2025

CI failed because Swift CI unconditionally enables all tests that are guarded by skipUnlessSupportedByToolchain (we don’t want to accidentally leave tests disabled in CI due to configuration issues). I suggest that we wait until swiftlang/swift#82464 is merged before merging this. Is that alright with you?

@a7medev
Copy link
Author

a7medev commented Jul 18, 2025

@ahoppen Yes, I think that makes sense.

I wonder where in the console output did you find this though 😅

Edit: I found it by downloading the plain text output and using grep to filter it out. Let me know if there's a more efficient way to do that though 😅

@ahoppen
Copy link
Member

ahoppen commented Jul 18, 2025

No, I always download the log files as well.

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo! This is awesome, thanks @a7medev!

As of now, SourceKit-LSP doesn't cache completion item documentation either, should we introduce a new full documentation cache (e.g. using LRUCache)?

It's probably worth merging this, seeing how it performs, and only adding the cache if we need it. If we do then it'll be a little more than just caching them as they're retrieved, since if that's too slow then getting them in the first place is as well 😅 (in which case we might need to pre-fetch a bunch).

@a7medev a7medev force-pushed the feat/full-documentation-in-code-completion branch from c0b4ca2 to 0bcd943 Compare August 16, 2025 21:15
@a7medev
Copy link
Author

a7medev commented Aug 16, 2025

We've found that the XML to markdown implementation has some problems.

Consider this example:

/// Adds two integers together
/// A really interesting function, no?
///
/// Usage:
/// ```swift
/// add(x: 14, y: 22)
/// ```
///
/// - Parameters:
///   - x: First arg
///   - y: Second arg
/// - Returns: Sum
func add(x: Int, y: Int) -> Int {
    return x + y
}

The full documentation for add is shown like this in VS Code:
Full documentation using XML to markdown conversion

this has some issues:

  1. Code blocks have line numbers and extra spacing before each line (prepended as part of the code to each line which can also cause issues with syntax highlighting) and there’s always an empty numbered line at the end which makes it even weirder. These line numbers are being added since the full XML output adds code between <zCodeLineNumbered> tags. When I checked Xcode’s documentation output, it didn’t actually include any line numbers at all, which I believe looks a lot nicer.
  2. Section headings are a bit large considering the area reserved for documentation output.
  3. The current XML to markdown implementation is incomplete (see Bullets in doc comments rendered as "Discussion," labels are dropped. #1014) which led to switching to the raw comment for the hover LSP request in Emit raw documentation comments instead of parsing XML #1091.

If we just output the raw comment instead, it looks like this. Which looks a lot nicer.
image

The downside of course is losing the structured format that the XML-based comment has so we should ultimately switch back to using the XML to markdown approach after improving it.

I agreed with @hamishknight on sticking to the raw comment for now to follow what hover is doing and to not block this on improving XML to markdown output.

@hamishknight @ahoppen @bnbarham Can you please re-review the PR after these changes? 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants